@uppy/companion-client: fix body/url on upload-success #4922
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Diff output filesdiff --git a/packages/@uppy/companion-client/lib/RequestClient.js b/packages/@uppy/companion-client/lib/RequestClient.js
index 8277e2f..2687682 100644
--- a/packages/@uppy/companion-client/lib/RequestClient.js
+++ b/packages/@uppy/companion-client/lib/RequestClient.js
@@ -376,9 +376,18 @@ async function _awaitRemoteFileUpload2(_ref4) {
break;
}
case "success": {
- var _socketAbortControlle2;
+ var _payload$response, _payload$response$sta, _payload$response2, _socketAbortControlle2;
+ const text = (_payload$response = payload.response) == null
+ ? void 0
+ : _payload$response.responseText;
this.uppy.emit("upload-success", this.uppy.getFile(file.id), {
uploadURL: payload.url,
+ status: (_payload$response$sta = (_payload$response2 = payload.response) == null
+ ? void 0
+ : _payload$response2.status) != null
+ ? _payload$response$sta
+ : 200,
+ body: text ? JSON.parse(text) : undefined,
});
(_socketAbortControlle2 = socketAbortController) == null || _socketAbortControlle2.abort == null
|| _socketAbortControlle2.abort(); |
|
Let us know how we can support. Would love to get this merged as its currently a blocker for us. |
|
I think we'll have it released before Friday :) |
* main: meta: disable `@typescript-eslint/no-non-null-assertion` lint rule (#4945) remove unnecessary `'use strict'` directives (#4943) @uppy/companion-client: type changes for provider-views (#4938) meta: bump ip from 1.1.8 to 1.1.9 (#4941) @uppy/companion-client: update types (#4927) Release: uppy@3.22.1 (#4935) update vi_VN translation (#4930) bump `@transloadit/prettier-bytes` (#4933) Release: uppy@3.22.0 (#4929) update `UppyFile` objects before emitting events (#4928) @uppy/transloadit: add `clientName` option (#4920) Fix broken previews after cropping (#4926) @uppy/compressor: upgrade compressorjs (#4924) Fix companion dns and allow redirects from http->https again (#4895) autoOpenFileEditor - rename "Edit file" to "Edit image" (#4925) meta: resolve jsx to Preact in shared tsconfig (#4923)
aduh95
approved these changes
Feb 22, 2024
Merged
github-actions bot
added a commit
that referenced
this pull request
Feb 22, 2024
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/audio | 1.1.7 | @uppy/react | 3.2.2 | | @uppy/companion | 4.12.3 | @uppy/status-bar | 3.2.8 | | @uppy/companion-client | 3.7.3 | @uppy/tus | 3.5.3 | | @uppy/core | 3.9.2 | @uppy/utils | 5.7.3 | | @uppy/dashboard | 3.7.4 | @uppy/xhr-upload | 3.6.3 | | @uppy/image-editor | 2.4.3 | uppy | 3.22.2 | - @uppy/core: fix plugin detection (Antoine du Hamel / #4951) - @uppy/core,@uppy/utils: Introduce `ValidateableFile` & move `MinimalRequiredUppyFile` into utils (Antoine du Hamel / #4944) - meta: uppy: fix bundle builder (Antoine du Hamel / #4950) - @uppy/core: improve `UIPluginOptions` types (Merlijn Vos / #4946) - @uppy/companion-client: fix body/url on upload-success (Merlijn Vos / #4922) - @uppy/utils: remove EventManager circular reference (Merlijn Vos / #4949) - @uppy/dashboard: MetaEditor + ImageEditor - new state machine logic (Evgenia Karunus / #4939) - meta: disable `@typescript-eslint/no-non-null-assertion` lint rule (Antoine du Hamel / #4945) - @uppy/companion-client: remove unnecessary `'use strict'` directives (Antoine du Hamel / #4943) - @uppy/companion-client: type changes for provider-views (Antoine du Hamel / #4938) - meta: bump ip from 1.1.8 to 1.1.9 (dependabot[bot] / #4941) - @uppy/companion-client: update types (Antoine du Hamel / #4927)
Murderlon
added a commit
that referenced
this pull request
Feb 26, 2024
* main: @uppy/file-input: refactor to TypeScript (#4954) Release: uppy@3.22.2 (#4952) @uppy/core: fix plugin detection (#4951) Introduce `ValidateableFile` & move `MinimalRequiredUppyFile` into utils (#4944) uppy: fix bundle builder (#4950) @uppy/core: improve `UIPluginOptions` types (#4946) @uppy/companion-client: fix body/url on upload-success (#4922) @uppy/utils: remove EventManager circular reference (#4949) MetaEditor + ImageEditor - new state machine logic (#4939) meta: disable `@typescript-eslint/no-non-null-assertion` lint rule (#4945) remove unnecessary `'use strict'` directives (#4943) @uppy/companion-client: type changes for provider-views (#4938) meta: bump ip from 1.1.8 to 1.1.9 (#4941)
mifi
added a commit
that referenced
this pull request
Feb 28, 2024
Merged
github-actions bot
added a commit
that referenced
this pull request
Feb 28, 2024
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/box | 2.2.1 | @uppy/onedrive | 3.2.1 | | @uppy/companion-client | 3.7.4 | @uppy/progress-bar | 3.1.0 | | @uppy/core | 3.9.3 | @uppy/provider-views | 3.10.0 | | @uppy/dashboard | 3.7.5 | @uppy/status-bar | 3.3.0 | | @uppy/file-input | 3.1.0 | @uppy/utils | 5.7.4 | | @uppy/form | 3.2.0 | @uppy/xhr-upload | 3.6.4 | | @uppy/image-editor | 2.4.4 | uppy | 3.23.0 | | @uppy/informer | 3.1.0 | | | - @uppy/form: migrate to TS (Merlijn Vos / #4937) - @uppy/box: fetchPreAuthToken in box too (Mikael Finstad / #4969) - @uppy/progress-bar: refactor to TypeScript (Mikael Finstad / #4921) - @uppy/onedrive: fix custom oauth2 credentials for onedrive (Mikael Finstad / #4968) - @uppy/companion-client,@uppy/utils,@uppy/xhr-upload: improvements for #4922 (Mikael Finstad / #4960) - @uppy/utils: fix various type issues (Mikael Finstad / #4958) - @uppy/provider-views: migrate to TS (Merlijn Vos / #4919) - @uppy/utils: simplify `findDOMElements` (Mikael Finstad / #4957) - @uppy/xhr-upload: fix getResponseData regression (Merlijn Vos / #4964) - @uppy/informer: migrate to TS (Merlijn Vos / #4967) - @uppy/core: remove unused import (Antoine du Hamel / #4972) - @uppy/image-editor: remove default target (Merlijn Vos / #4966) - @uppy/angular: Build fixes (Mikael Finstad / #4959) - meta: Fix flaky e2e test (Murderlon) - meta: fix e2e flake (Mikael Finstad / #4961) - meta: add support for `Fragment` short syntax (Antoine du Hamel / #4953) - @uppy/file-input: refactor to TypeScript (Antoine du Hamel / #4954)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4915
Fixes #4909
Not the cleanest fix, but not sure how do it otherwise. Couple of problems:
urlback from CompanionresponseandbytesUploaded. Insideresponsethere isresponseText,status,headers.getResponseData,getResponseError, andresponseUrlFieldNameto extract data for local files, these options are not used for responses from Companion as they are handled inside companion-client. Those options where introduced for people who want to convert XML to JSON (can't believe we support that 🙈). Funnily enough, those people with XML were never able to use Companion as Companion expects the response to be JSON before sending it over websockets.upload-successbecause companion-client has no knowledge of that. Nor can we type it properly internally.So since these options we don't assume the response and where to find the upload URL, you can extract it / point to it. How do we do it in for Companion uploads then? Well we either guess on the client of guess inside companion. Currently Companion never sends the upload URL back:
uppy/packages/@uppy/companion/src/server/Uploader.js
Lines 645 to 648 in 3c0140a
But it does for tus/S3. Since Companion is responsible for sending back the upload url for other uploaders I decided to guess inside Companion rather than on the client.
What's the ideal fix? In 4.0 I think we should remove all these extraction options for xhr-upload and force people to return JSON with a
urlproperty. Realistically, if you run your own server you need to create a new endpoint for Uppy anyway, so why make your life difficult with XML? Or by returning a key different fromurl? I think we can easily enforce that.Then ideally
upload-successreturns consistently, so not all kinds of different data for different uploaders. Not sure how that's possible without a deep dive.